refactor: rename minimum partition ID constant to be more generic #34529
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @mariajgrimaldi! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
| @@ -17,7 +17,7 @@ | |||
| # ID is stored in the xblock group_access dict). | |||
| ENROLLMENT_TRACK_PARTITION_ID = 50 | |||
|
|
|||
| MINIMUM_STATIC_PARTITION_ID = 100 | |||
| MINIMUM_UNUSED_PARTITION_ID = 100 | |||
There was a problem hiding this comment.
# Each user partition has an ID that is unique within its learning context.
# The IDs must be valid MySQL primary keys, ie positive integers 1 -> 2^31-1.
# We must carefully manage these IDs, because once they are saved to OLX and the db, they cannot change.
# Here is how we delegate the ID range:
# * 1 -> 49: Unused/Reserved
# * 50: The enrollment track partition
# * 51: The content type gating partition (defined elsewhere)
# * 52-99: Available for other single user partitions, plugged in via setup.py.
# Operators, beware of conflicting IDs between plugins!
# * 100 -> 2^31-1: General namespace for generating IDs at runtime.
# This includes, at least: content partitions, the cohort partition, and teamset partitions.
# When using this range, user partition implementations must check to see that they
# are not conflicting with an existing partition for the course.
ENROLLMENT_TRACK_PARTITION_ID = 50
MINIMUM_UNUSED_PARTITION_ID = 100To make the new name clearer, I've rewritten the comment above. What do you think?
There was a problem hiding this comment.
That's much better! Thank you for all the suggestions.
mariajgrimaldi
force-pushed
the
MJG/rename-min-partition-constant
branch
from
d5fcea2
to
2d0b78b
Compare
|
Hi @kdmccormick! Is this something you can merge (if it's all set)? |
|
@mphilbrick211: I can merge this early tomorrow. EDIT: on Monday! 😅 |
Rename MINIMUM_STATIC_PARTITION_ID to MINIMUM_UNUSED_PARTITION_ID so it's not confusing when used to generate IDs for static or dynamic partitions.
mariajgrimaldi
force-pushed
the
MJG/rename-min-partition-constant
branch
from
2d0b78b
to
3b10d4c
Compare
|
@mariajgrimaldi 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
|
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
|
2U Release Notice: This PR has been deployed to the edX production environment. |
Description
Rename MINIMUM_STATIC_PARTITION_ID to MINIMUM_UNUSED_PARTITION_ID so it's not confusing when used to generate IDs for static or dynamic partitions.
Supporting information
See: #33788